Skip to content

feat(chat): add /detect slash command for mid-session re-detection#102

Open
michaelroy-amd wants to merge 2 commits into
mainfrom
feat/chat-redetect
Open

feat(chat): add /detect slash command for mid-session re-detection#102
michaelroy-amd wants to merge 2 commits into
mainfrom
feat/chat-redetect

Conversation

@michaelroy-amd

@michaelroy-amd michaelroy-amd commented Jul 11, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds a /detect [accept|save|dismiss] slash command so a local chat
    engine can be re-detected after ChatConsent::Accepted, without a
    dedicated keybinding colliding with focused chat text entry.
  • Bare /detect re-runs the same probe used at startup
    (detect_local_chat: managed-services registry, then Lemonade
    :13305 / vLLM :8000 / rocm serve :11435) and echoes the result into
    the chat transcript.
  • /detect accept / /detect save / /detect dismiss route through
    the existing accept_detect_offer / save_detect_offer /
    dismiss_detect_offer reducer methods.

Root cause

Once chat is ChatConsent::Accepted, the pre-accept 'd' gate key is
unreachable: while a text field is focused a bare d keypress is
ordinary chat input, and when unfocused the global hotkey table does
not bind 'd' for the Chat tab. There was no way to react to a local
engine coming online (or changing) after the initial handshake short
of restarting the app.

Fix

  • slash.rs: new "detect" branch dispatches to the pre-existing
    AppState reducer methods used by the pre-accept offer UI, so no
    new detection or endpoint-swap plumbing was introduced.
  • app/mod.rs:
    • request_detect() now also echoes a "Detecting…" message into the
      transcript when already Accepted (previously this messaging only
      made sense pre-accept, driving the gate UI).
    • set_detect_result() echoes the found/not-found result into the
      transcript when Accepted, since draw() only renders the
      offer y/s/n prompt pre-accept (confirmed in ui/tabs/chat.rs) —
      without this the probe result would be silently discarded once
      accepted.
  • Accepting or saving a re-detected endpoint via /detect accept /
    /detect save goes through the same accept_detect_offer /
    save_detect_offer calls the initial pre-accept flow uses, so it
    raises the chat_endpoint_rebuild dispatch edge (introduced in
    PR fix(dash): rebuild live chat agent when a local endpoint is accepted #81) exactly like the initial accept does, swapping the live
    agent to the newly detected endpoint.

Coordination notes

Relates to EAI-7354.

Test Plan

  • cargo build -p rocm-dash-tui
  • cargo clippy -p rocm-dash-tui --all-targets --all-features (clean, -D warnings)
  • cargo test -p rocm-dash-tui -- --test-threads=1 (551 passed, 0 failed)
  • New tests:
    • slash_detect_probes_and_echoes_result_while_accepted
    • slash_detect_accept_raises_endpoint_rebuild_like_initial_accept
    • slash_detect_dismiss_and_unknown_subcommand

@michaelroy-amd
feat(chat): add /detect slash command for mid-session re-detection
c486146 
Once ChatConsent::Accepted, the pre-accept 'd' gate key is unreachable
(it's ordinary chat text when focused, and unbound when unfocused), so
there was no way to re-run local-engine detection after the initial
handshake without restarting the app.

Add a `/detect [accept|save|dismiss]` slash command that reuses the
existing request_detect/accept_detect_offer/save_detect_offer/
dismiss_detect_offer reducer methods. A bare `/detect` raises the same
probe used at startup and echoes the result into the chat transcript
(the offer y/s/n UI is never drawn once Accepted, since draw() only
renders it pre-accept). `/detect accept` and `/detect save` route
through the same code path as the initial accept, so a re-detect mid-
session raises the chat_endpoint_rebuild edge exactly like startup
does, swapping the live agent to the newly detected endpoint.

Relates to EAI-7354.

Signed-off-by: Michael Roy <michael.roy@amd.com>
@michaelroy-amd michaelroy-amd mentioned this pull request Jul 11, 2026
Open
4 tasks
@michaelroy-amd
feat(chat): make /detect sub-commands case-insensitive and hint on no…
15979fa 
… offer

Two consistency refinements to the /detect slash command:

- Lowercase the sub-command before matching, so `/detect ACCEPT` etc.
  behave like the lowercase forms — mirroring the case-insensitive
  parsing of the sibling `/permissions` and `/provider` commands.

- Emit a short hint turn when `/detect accept` or `/detect save` is
  used with no pending detected endpoint (previously a silent reducer
  no-op), pointing the user at bare `/detect` to probe first.

Relates to EAI-7354.

Signed-off-by: Michael Roy <michael.roy@amd.com>
rominf
rominf approved these changes Jul 13, 2026
View reviewed changes

@rominf rominf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. /detect dispatch/parsing mirrors sibling slash commands (case-insensitive sub-command via split_whitespace().nth(1), whitespace-robust), and it routes straight through the existing accept_detect_offer / save_detect_offer / dismiss_detect_offer reducers rather than reimplementing detection. /detect accept raises chat_endpoint_rebuild identically to the initial pre-accept accept (asserted by test). The feature is correctly scoped to post-Accepted state.

Minor UX nit: a bare /detect while an offer is already pending is a silent no-op (early return on chat_detect_offer.is_some()) with no on-screen feedback post-accept — a one-line "an endpoint is already pending — /detect accept|dismiss first" hint would close it.

Approving.

@volen-silo

volen-silo commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

🔴 Automated review · pr-review-watcher · 15979fa

Summary

Adds a /detect [accept|save|dismiss] slash command so a local chat engine can be re-detected mid-session once ChatConsent::Accepted (where the pre-accept 'd' gate key is unreachable), routing accept/save/dismiss through the existing reducer methods so a re-detect+accept raises the chat_endpoint_rebuild edge exactly like the initial accept. Verdict: Approve. Verified: cargo test -p rocm-dash-tui builds clean and passes (569 tests, 0 failed; 5 new detect tests green), cargo clippy --all-targets --all-features clean; confirmed /detect is only reachable once Accepted (input is unfocusable/unsubmittable before then, so the consent-branching in request_detect/set_detect_result is correct defensive code), the offer/gate UI (ui/tabs/chat.rs draw_consent) only renders pre-accept so echoing results into the transcript is the right surface, and /detect accept correctly re-raises the endpoint-rebuild edge. Blocking: 0 · Non-blocking: 3.

🚫 Blocking (must fix before merge)

None.

Non-blocking

  • app/mod.rs:2176-2187 — /detect save gives no user-visible feedback once Accepted: the persist success path is tracing::info! only, and the failure path sets chat_detect_msg, which is a dead write post-accept (only rendered by draw_consent, gated to != Accepted in ui/tabs/chat.rs:114). Consider pushing a ChatTurn in both the Ok/Err arms.
  • app/mod.rs:1066-1080 — /detect accept/save push no acceptance confirmation into the transcript; the endpoint silently switches. A one-line "switched to " echo would match the discoverability of the rest of the flow.
  • app/mod.rs:1012-1015 — a bare /detect while a probe is in flight or an offer is already pending is a silent no-op (early return, no hint), unlike the /detect accept-with-no-offer path which does emit a hint. Minor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rominf rominf rominf approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@michaelroy-amd @volen-silo @rominf